Add tests for switch_working_directory helper#10766
Add tests for switch_working_directory helper#10766radoering merged 6 commits intopython-poetry:mainfrom
switch_working_directory helper#10766Conversation
Reviewer's GuideThis PR adds two pytest-based unit tests for the switch_working_directory helper to verify it correctly restores the original working directory on exceptions and optionally deletes the temporary directory when requested. File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Follow-up pushed in Change
Targeted validation
|
|
Pushed follow-up commit 0facb5c to fix the failing What changed (auto-fixes from the repo hooks):
Validation:
|
radoering
left a comment
There was a problem hiding this comment.
The two tests seem arbitrary. A complete test should test both aspects (cwd is changed (back) and cwd is only deleted if remove=True) - both under normal conditions and if there is an error. I think one parameterized test makes sense.
|
Thanks for the clear review guidance. I’ll rework this into more complete coverage: both normal/error paths, and delete behavior for |
|
Thanks for the concrete feedback — I pushed a follow-up in commit aa866ed that rewrites the coverage into one parametrized test over (remove, raise_error). What this now verifies in one place:
Targeted validation run locally:
If you want, I can split this back into two parametrized tests (restore/remove) for readability, but I kept it as one per your suggestion. |
|
Pushed follow-up commit edc596c to address the failing pre-commit.ci check.\n\n### What changed\n- Reformatted the long pytest.mark.parametrize decorator in ests/test_helpers.py (Ruff format).\n- Reworked the nested with in the error-path branch into a single multi-context with (...) block to satisfy Ruff SIM117.\n\n### Validation\n- python -m pre_commit run --files tests/test_helpers.py ✅\n |
|
Quick follow-up on the requested rework: the PR now has a single parametrized test covering both normal/error paths and remove=true/false behavior, plus a pre-commit clean-up in edc596c.\n\n@radoering when you have a moment, could you take another look and let me know if this now matches what you had in mind? |
edc596c to
5623aae
Compare
|
Follow-up pushed: I rebased this branch onto the current main to clear the BEHIND state after the latest upstream changes.\n\nValidation after rebase:\n- python -m pytest --noconftest tests/test_helpers.py -k switch_working_directory -q ✅ (4 passed)\n\n@radoering when you have time, could you take another look at the latest revision? |
|
Thanks for the detailed review. I updated the tests into a single parameterized matrix to cover both behaviors (cwd restore + conditional directory removal) across normal and error paths.\n\nIf you have a minute, could you please take another look and let me know if you’d like this split differently? |
switch_working_directory helper
Summary
switch_working_directory()restores the original cwd when an exception is raisedremove=Truedeletes the temporary working directory after exiting the context managerRelated to #9161
Summary by Sourcery
Add coverage for the switch_working_directory helper behavior.
Tests: